Skip to content

Conversation

dimpase
Copy link
Member

@dimpase dimpase commented Aug 27, 2025

10.8.beta0 broke installation of sagemath's jupyter kernel, apparently by upgrading setuptools (sic!),
see #40586 (comment)
and #40633

Here we implement the explicit installation of the kernel in the classic build, and make it compatible with meson

📝 Checklist

  • The title is concise and informative.
  • The description explains in detail what this PR is about.
  • I have linked a relevant issue or discussion.
  • I have created tests covering the changes.
  • I have updated the documentation and checked the documentation preview.

⌛ Dependencies

#39030 - meson build for sagelib

@dimpase dimpase requested a review from tobiasdiez August 27, 2025 17:24
@dimpase
Copy link
Member Author

dimpase commented Aug 27, 2025

as #40633 was merged about an hour ago, this will need to revert reverts done there, once the beta1 dust settles

@dimpase dimpase requested a review from kiwifb August 27, 2025 17:26
@dimpase
Copy link
Member Author

dimpase commented Aug 27, 2025

here ./configure installs the kernel data (filling the version in) into local/share/notebook/

Copy link

github-actions bot commented Aug 27, 2025

Documentation preview for this PR (built with commit 7df7430; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

As well, we make consistent use of PACKAGE_VERSION (instead of
SAGE_VERSION) in meson, and rename PACKAGE_VERSION in meson.build's to something
more descriptive and non-clashing
@tobiasdiez
Copy link
Contributor

#39030 fixes this as well, or not?

@dimpase
Copy link
Member Author

dimpase commented Aug 28, 2025

@tobiasdiez I think by default jupyter should be called with --sys-prefix rather than --user in #39030
--user is somewhat arbitrary, whereas --sys-prefix makes sure the jupyter kernel gets installed in the same venv as the one that is being used.

In a different scenario, with external jupyter, --user might be useful, but this is something to sort out.

As well, in sage-distro the version of sagelib is PACKAGE_VERSION, so the renaming of the variables in the branch makes sense too (we don't want different kernel.json.in for different install scenarios).

I must say I don't know what your version setting to 1.2.3 is meant to do, by the way

@kiwifb
Copy link
Member

kiwifb commented Aug 28, 2025

Certainly using --user would make it unsuitable for installing system wide. By default, I am expecting --user to put things in the .local/share folder of the user running the install.

@dimpase dimpase mentioned this pull request Aug 28, 2025
5 tasks
@@ -12,7 +12,7 @@ mpfr = dependency('mpfr', version: '>= 4.0.1')

# Configuration data
conf = configuration_data()
conf.set('PACKAGE_VERSION', '"' + meson.project_version() + '"')
conf.set('SAGE_MESON_PKG_VERSION', '"' + meson.project_version() + '"')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's the version of mpfi, you probably don't want to change that.

@@ -2,7 +2,7 @@ sage_install_dir = py.get_install_dir() / 'sage'

# Generate the configuration file
conf_data = configuration_data()
conf_data.set('PACKAGE_VERSION', '1.2.3')
conf_data.set('SAGE_MESON_PKG_VERSION', '1.2.3')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can just delete that line along with it's usage in _conf.py.in. No idea about it's purpose, if it ever served one.

@tobiasdiez
Copy link
Contributor

Meson should already install the jupyter kernel in whatever python venv it ends up being installed. The workaround in #39030 was only for editable installs where this was not working correctly. Not sure if user or prefix is better for such editable installs.

@dimpase
Copy link
Member Author

dimpase commented Aug 28, 2025

Certainly using --user would make it unsuitable for installing system wide. By default, I am expecting --user to put things in the .local/share folder of the user running the install.

@tobiasdiez @kiwifb
I think it should be configurable. --user would work for the external jupyter, otherwise it's iffy.

@dimpase
Copy link
Member Author

dimpase commented Aug 28, 2025

Meson should already install the jupyter kernel in whatever python venv it ends up being installed.

where in the code does this happen?

@kwankyu
Copy link
Collaborator

kwankyu commented Aug 28, 2025

#40700 notes that using --user for the kernel installation option is not always what's needed

So?

According to my tests, the branch here does not fix the broken live doc (broken perhaps because of missing sagemath kernel).

@kwankyu
Copy link
Collaborator

kwankyu commented Aug 28, 2025

Downgrading setuptools does not work either in meson build. Recall that it worked with make build.

@kwankyu
Copy link
Collaborator

kwankyu commented Aug 28, 2025

I wonder if you are aware that src/sage_setup/command/sage_install.py is responsible in installing sagemath kernel. I suspect that that is not working with the upgraded setuptools.

But I don't know about the meson side.

@dimpase
Copy link
Member Author

dimpase commented Aug 28, 2025

sage_setup should not be needed in the meson build, as far as I understand - right, @tobiasdiez ?

note that the explicit jupyter kernelspec call is only in one branch (editable install ?) of sagelib/spkg-install - so it could be the case it's not even called.

@dimpase
Copy link
Member Author

dimpase commented Aug 28, 2025

SageKernelSpec class is 10 years old and probably hopelessly outdated.

I guess modern Jupyter, which has evolved a lot, needs very little there.

@dimpase
Copy link
Member Author

dimpase commented Aug 28, 2025

Downgrading setuptools does not work either in meson build. Recall that it worked with make build.

meson build in #39030 uses --user option, meaning that the kernel might end up somewhere in ~/ rather than in the place needed.

@tobiasdiez
Copy link
Contributor

sage_setup should not be needed in the meson build, as far as I understand - right, @tobiasdiez ?

Yes, it's not used for meson and can be deleted after we fully migrated to meson.

This is how the Jupyter kernel is installed with meson:

sage/src/sage/meson.build

Lines 140 to 154 in e656b82

# Jupyter kernel spec
# meson-python maps 'datadir' currently the python prefix, see
# https://github.com/mesonbuild/meson-python/issues/517
kernel_dir = get_option('datadir') / 'share/jupyter/kernels/sagemath'
install_data('ext_data/notebook-ipython/logo.svg', install_dir: kernel_dir)
install_data('ext_data/notebook-ipython/logo-64x64.png', install_dir: kernel_dir)
kernel_data = configuration_data()
kernel_data.set('SAGE_VERSION', meson.project_version())
configure_file(
input: 'ext_data/notebook-ipython/kernel.json.in',
output: 'kernel.json',
install_dir: kernel_dir,
install: true,
configuration: kernel_data,
)

As for the process, are you okay waiting for #39030 to be merged first? Then we can iterate more easily here on the jupyter kernel installation and make sure it doesn't break in one of the 4 supported build methods (plain meson/sage-distro + editable/non-editable).

@dimpase
Copy link
Member Author

dimpase commented Aug 29, 2025

@tobiasdiez - absolutely, #39030 should be 1st.

Actually, quite a bit of stuff dealing with Jupiter in Sage is totally antique - what nowadays is done by 1-liner jupyter kernelspec... takes many pages of code. (SageKernelSpec class etc)

@egourgoulhon
Copy link
Member

Sorry I am too unfamiliar with the meson build to be of any help here. Simply, may I suggest that when testing the SageMath kernel, you run Jupyterlab on this notebook: https://github.com/egourgoulhon/SageMathTest/blob/master/Notebooks/test_display.ipynb
Thanks!

@dimpase
Copy link
Member Author

dimpase commented Aug 29, 2025

Sorry I am too unfamiliar with the meson build to be of any help here. Simply, may I suggest that when testing the SageMath kernel, you run Jupyterlab on this notebook: https://github.com/egourgoulhon/SageMathTest/blob/master/Notebooks/test_display.ipynb Thanks!

Do you mean Sage's Jupyterlab? External Jupyterlab?

@dimpase dimpase marked this pull request as draft August 29, 2025 19:10
@egourgoulhon
Copy link
Member

Sorry I am too unfamiliar with the meson build to be of any help here. Simply, may I suggest that when testing the SageMath kernel, you run Jupyterlab on this notebook: https://github.com/egourgoulhon/SageMathTest/blob/master/Notebooks/test_display.ipynb Thanks!

Do you mean Sage's Jupyterlab? External Jupyterlab?

A priori Sage's Jupyterlab, but it would be nice if the built SageMath kernel works in the system's Jupyterlab as well.

@dimpase
Copy link
Member Author

dimpase commented Aug 30, 2025

this notebook basically works fine, save for the test to show a pdf in an external viewer.
The error appears to be that sage.pdf is saved in the wrong place.

@enriqueartal
Copy link
Contributor

enriqueartal commented Aug 30, 2025

I have tested these changes in Fedora 42 (classic build). It works with sage -n jupyterlab (except for the launcher, but it may be a local issue). I linked the kernel to a place where external jupyterlab can found this kernel but it cannot connect notebooks to this kernel:

[I 2025-08-30 09:56:54.198 ServerApp] AsyncIOLoopKernelRestarter: restarting kernel (4/5), new random ports
/usr/bin/python3: Error while finding module specification for 'sage.repl.ipython_kernel' (ModuleNotFoundError: No module named 'sage')

PS. And make build always rebuild the whole sagelib. Actually, running again I get:

[sagelib-10.8.beta1] [spkg-install] Successfully installed sagemath-standard-10.8b1
[sagelib-10.8.beta1] [spkg-install] Traceback (most recent call last):
[sagelib-10.8.beta1] [spkg-install]   File "/usr/local/sage/local/var/lib/sage/venv-python3.13/bin/jupyter-kernelspec", line 8, in <module>
[sagelib-10.8.beta1] [spkg-install]     sys.exit(KernelSpecApp.launch_instance())
[sagelib-10.8.beta1] [spkg-install]              ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^
[sagelib-10.8.beta1] [spkg-install]   File "/usr/local/sage/local/var/lib/sage/venv-python3.13/lib64/python3.13/site-packages/traitlets/config/application.py", line 1075, in launch_instance
[sagelib-10.8.beta1] [spkg-install]     app.start()
[sagelib-10.8.beta1] [spkg-install]     ~~~~~~~~~^^
[sagelib-10.8.beta1] [spkg-install]   File "/usr/local/sage/local/var/lib/sage/venv-python3.13/lib64/python3.13/site-packages/jupyter_client/kernelspecapp.py", line 338, in start
[sagelib-10.8.beta1] [spkg-install]     return self.subapp.start()
[sagelib-10.8.beta1] [spkg-install]            ~~~~~~~~~~~~~~~~~^^
[sagelib-10.8.beta1] [spkg-install]   File "/usr/local/sage/local/var/lib/sage/venv-python3.13/lib64/python3.13/site-packages/jupyter_client/kernelspecapp.py", line 151, in start
[sagelib-10.8.beta1] [spkg-install]     self.kernel_spec_manager.install_kernel_spec(
[sagelib-10.8.beta1] [spkg-install]     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^
[sagelib-10.8.beta1] [spkg-install]         self.sourcedir,
[sagelib-10.8.beta1] [spkg-install]         ^^^^^^^^^^^^^^^
[sagelib-10.8.beta1] [spkg-install]     ...<3 lines>...
[sagelib-10.8.beta1] [spkg-install]         replace=self.replace,
[sagelib-10.8.beta1] [spkg-install]         ^^^^^^^^^^^^^^^^^^^^^
[sagelib-10.8.beta1] [spkg-install]     )
[sagelib-10.8.beta1] [spkg-install]     ^
[sagelib-10.8.beta1] [spkg-install]   File "/usr/local/sage/local/var/lib/sage/venv-python3.13/lib64/python3.13/site-packages/jupyter_client/kernelspec.py", line 401, in install_kernel_spec
[sagelib-10.8.beta1] [spkg-install]     shutil.copytree(source_dir, destination)
[sagelib-10.8.beta1] [spkg-install]     ~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^
[sagelib-10.8.beta1] [spkg-install]   File "/usr/lib64/python3.13/shutil.py", line 591, in copytree
[sagelib-10.8.beta1] [spkg-install]     with os.scandir(src) as itr:
[sagelib-10.8.beta1] [spkg-install]          ~~~~~~~~~~^^^^^
[sagelib-10.8.beta1] [spkg-install] FileNotFoundError: [Errno 2] No such file or directory: '/share/notebook'
[sagelib-10.8.beta1] Moving package files from temporary location /usr/local/sage/local/var/lib/sage/venv-python3.13/var/tmp/sage/build/sagelib-10.8.beta1/inst to /usr/local/sage/local/var/lib/sage/venv-python3.13

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants